• Create ticket/issue

Follow the guide to enable codebase context checks.

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logging: The automatic mutation of settings post_menu and post_menu_hidden_items when
content_localization_enabled becomes true is not accompanied by any audit/event logging to
record who triggered the change and what was changed.

if name == :content_localization_enabled && new_value == true
  %i[post_menu post_menu_hidden_items].each do |setting_name|
    current_items = SiteSetting.get(setting_name).split("|")
    if current_items.exclude?("addTranslation")
      edit_index = current_items.index("edit")
      insert_position = edit_index ? edit_index + 1 : 0
      current_items.insert(insert_position, "addTranslation")
      SiteSetting.set(setting_name, current_items.join("|"))
    end
  end
end

Learn more about managing compliance generic rules or creating your own custom rules

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Edge cases unhandled: The code assumes SiteSetting.get(setting_name) returns a non-nil string and splits on
"|" without guarding against nil/blank or malformed values, and updates settings
without handling potential failures.

current_items = SiteSetting.get(setting_name).split("|")
if current_items.exclude?("addTranslation")
  edit_index = current_items.index("edit")
  insert_position = edit_index ? edit_index + 1 : 0
  current_items.insert(insert_position, "addTranslation")
  SiteSetting.set(setting_name, current_items.join("|"))
end

Learn more about managing compliance generic rules or creating your own custom rules

Instead of modifying the post_menu settings as a side-effect when
content_localization_enabled is toggled, the 'addTranslation' button should be
conditionally rendered in the UI. This avoids unexpected changes to stored
configurations and handles cases where the feature is disabled.

Examples:

  if name == :content_localization_enabled && new_value == true
    %i[post_menu post_menu_hidden_items].each do |setting_name|
      current_items = SiteSetting.get(setting_name).split("|")
      if current_items.exclude?("addTranslation")
        edit_index = current_items.index("edit")
        insert_position = edit_index ? edit_index + 1 : 0
        current_items.insert(insert_position, "addTranslation")
        SiteSetting.set(setting_name, current_items.join("|"))
      end
    end

 ... (clipped 1 lines)

Solution Walkthrough:

Before:

# config/initializers/014-track-setting-changes.rb

DiscourseEvent.on(:site_setting_changed) do |name, old_value, new_value|
  if name == :content_localization_enabled && new_value == true
    %i[post_menu post_menu_hidden_items].each do |setting_name|
      current_items = SiteSetting.get(setting_name).split("|")
      if current_items.exclude?("addTranslation")
        # ... logic to find insert position
        current_items.insert(insert_position, "addTranslation")
        SiteSetting.set(setting_name, current_items.join("|"))
      end
    end
  end
end

After:

# config/initializers/014-track-setting-changes.rb
# The entire block for handling :content_localization_enabled would be removed.

# In the relevant frontend component (conceptual):
function getPostMenuItems() {
  let menuItems = SiteSettings.post_menu.split("|");
  if (SiteSettings.content_localization_enabled) {
    if (!menuItems.includes("addTranslation")) {
      // ... logic to find insert position
      menuItems.insert(insertPosition, "addTranslation");
    }
  } else {
    // This part is missing in the current PR
    menuItems = menuItems.filter(item => item !== "addTranslation");
  }
  return menuItems;
}

Add .to_s before calling .split("|") on the result of
SiteSetting.get(setting_name) to prevent a potential crash if the setting value
is nil.

config/initializers/014-track-setting-changes.rb [93]

-current_items = SiteSetting.get(setting_name).split("|")
+current_items = SiteSetting.get(setting_name).to_s.split("|")

• Apply / Chat